fix: Fix import paths with site-packages as part of package name#756
Draft
last-partizan wants to merge 4 commits intopython-rope:masterfrom
Draft
fix: Fix import paths with site-packages as part of package name#756last-partizan wants to merge 4 commits intopython-rope:masterfrom
site-packages as part of package name#756last-partizan wants to merge 4 commits intopython-rope:masterfrom
Conversation
last-partizan
commented
Feb 25, 2024
| pass | ||
| else: | ||
| # If path includes "site-packages", we're interested in part after this. | ||
| rel_path_parts = rel_path_parts[site_packages_index + 1:] |
Contributor
Author
There was a problem hiding this comment.
That's temporary code from my first attempt, but we probably need to leave here a check for no "site-packages" in path. Just in case.
last-partizan
commented
Feb 25, 2024
Comment on lines
+228
to
+237
| extra_ignores = [ | ||
| p.pathlib.relative_to(self.root.pathlib) | ||
| for p in self.get_python_path_folders() | ||
| if p.pathlib.is_relative_to(self.root.pathlib) | ||
| and "site-packages" in p.pathlib.parts | ||
| ] | ||
| for p in extra_ignores: | ||
| # I'm using this approach, because self.prefs.add after _init_prefs | ||
| # does not work for some reason. | ||
| prefs["ignored_resources"].append(str(p)) |
Contributor
Author
There was a problem hiding this comment.
That's my second attempt, and i need feedback here.
Is it good approach, or maybe some other ideas?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This is another fix, related to #722, i just started using
python-lsp-serverandropefor autoimports, and noticed it includes.direnv.python-3.11.lib.python3.11as part of module name when importing.We need to cut part of the path to site-packages, to generate correct import name.That was my first attempt to fix this, but turned that wasn't a problem. The problem was, in my case - that local venv
.venvwas included in project resources.As far as i understand, we have two caches
generate_cachecreates project-specific cachegenerate_module_cachecreates cache of system/venv modules.And when .venv is in the same dir as a project, it was included in project files.
My current first attempt just filters out path if it has "site-packages" in it, but, maybe we need smarter solution, for example:
get_python_filesshould exclude any file fromget_python_path_folders(except project root).For this i need some guidance, @tkrabel @lieryan
Checklist (delete if not relevant):